-
Notifications
You must be signed in to change notification settings - Fork 29
🤖 feat: add UI styling for background bash processes #923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
aaaf93f to
0a76e44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
9ddd37c to
d8dc85b
Compare
Adds run_in_background=true option to bash tool for long-running processes
(dev servers, builds, file watchers). Returns process ID and output file
paths for agents to monitor via tail/grep/cat.
Key features:
- Works on both local and SSH runtimes (unified setsid+nohup pattern)
- Output written to files: /tmp/mux-bashes/{workspaceId}/{processId}/
- Exit code captured via bash trap to exit_code file
- Process group termination (SIGTERM → wait → SIGKILL)
- meta.json tracks process metadata
Tools:
- bash(run_in_background=true) - spawns process, returns stdout_path/stderr_path
- bash_background_list - lists processes with file paths
- bash_background_terminate - kills process group
Removed bash_background_read (agents read files directly with bash).
_Generated with mux_
d8dc85b to
49b07ef
Compare
- Add NON_INTERACTIVE_ENV_VARS to SSH background spawns (parity with local) - Use expanded paths in buildSpawnCommand so tilde resolves correctly
Handle is always set on spawn and never cleared, so the null type and guards were dead code from an unimplemented recovery feature.
Replace setTimeout(500) with polling helpers that retry until output appears or exit code is set. Handles SSH latency variability without making tests slower than necessary.
The earlier fix incorrectly passed expandTildeForSSH paths (already quoted) to buildSpawnCommand which quotes again via shellQuote, breaking redirects. Raw paths are correct - buildSpawnCommand handles quoting internally.
Use os.tmpdir() instead of hardcoded /tmp/mux-bashes. On Windows, convert to POSIX path using cygpath for Git Bash compatibility. This fixes background process execution on Windows where /tmp doesn't exist or requires elevated privileges.
Add quotePath parameter to buildSpawnCommand allowing callers to choose quoting strategy. SSH now uses expandTildeForSSH which produces double- quoted paths with $HOME expansion, fixing ~/... bgOutputDir configs. Local runtime continues using shellQuote (single quotes) as default.
Add shell option to execAsync so callers can specify bash for POSIX commands that don't work with cmd.exe (Windows default). Fixed: - LocalBaseRuntime.spawnBackground: nohup/setsid commands - WorktreeRuntime.deleteWorkspace: rm -rf command
- Quote bash path in buildSpawnCommand to handle Windows paths with spaces (e.g., C:\Program Files\Git\bin\bash.exe) - Add cwd validation to SSH spawnBackground (parity with local runtime) - Add test for bash paths with spaces
Add toPosixPath() utility that uses cygpath to convert Windows paths (C:\foo\bar) to POSIX format (/c/foo/bar) for Git Bash compatibility. Apply to: - LocalBaseRuntime.spawnBackground: exitCodePath, cwd, stdoutPath, stderrPath - WorktreeRuntime.deleteWorkspace: deletedPath for rm -rf - runtimeFactory.getDefaultBgOutputDir: refactored to use shared utility
- getDefaultBgOutputDir: return native path for Node fs APIs, POSIX conversion happens at shell command construction time - LocalBackgroundHandle.terminate: run winpid lookup via bash since /proc/PID/winpid is MSYS2's virtual filesystem, not visible to Node.js - Add toPosixPath tests for path conversion behavior - Add Windows POSIX path handling tests to backgroundCommands.test.ts
buildTerminateCommand now accepts optional quotePath parameter (like buildSpawnCommand) to avoid double-quoting when used with SSH paths. SSHBackgroundHandle.terminate() passes raw path + expandTildeForSSH instead of pre-expanded path that would get quoted again by shellQuote.
8920d04 to
8129746
Compare
8129746 to
2c09936
Compare
d8dc85b to
7cb5d24
Compare
7cb5d24 to
fc3f359
Compare
- Replace platform-specific PGID detection with universal fallback chain: ps -o pgid= → /proc/$!/pgid → PID (works on Linux, macOS, MSYS2) - Use numeric signals (-15, -9) instead of named (-TERM, -KILL) for MSYS2 compat - Extract parsePidPgid() helper to backgroundCommands.ts (DRY) - Simplify LocalBackgroundHandle.terminate() from ~90 to ~17 lines - Remove Windows-specific conditional code - Now uses buildTerminateCommand via bash shell (same as SSH) - Add PGID parameter to SSHBackgroundHandle for proper process group termination _Generated with mux_
08cc49c to
6b0c881
Compare
Without job control, backgrounded processes inherit the parent's PGID. On Linux locally, this caused 'kill -PGID' to kill mux itself. Adding 'set -m' enables bash job control, which creates a new process group for backgrounded processes (PID === PGID), making it safe to kill. _Generated with mux_
With set -m, PID === PGID is guaranteed (process is its own group leader).
Remove the redundant PGID lookup chain (ps → /proc → fallback) and use
just the PID throughout.
- buildSpawnCommand: echo $! instead of PGID lookup
- parsePidPgid → parsePid: return number instead of {pid, pgid}
- Remove pgid field from LocalBackgroundHandle and SSHBackgroundHandle
- Clean up tests: 46 → 20 tests, remove redundant assertions
The EXIT trap in the process writes $? (which is 0 for SIGTERM) and would overwrite our exit code if we wrote it immediately. Now we wait for the process to exit before writing 143 (SIGTERM) or 137 (SIGKILL).
kill -0 ${pid} only checks if the parent is alive, but children may
survive SIGTERM. Changed to kill -0 ${negPid} to check if any process
in the group is still alive before deciding to send SIGKILL.
6b0c881 to
f463c75
Compare
- Use execFileSync with args array in toPosixPath to avoid shell injection - Add 5s timeout to app dispose to ensure quit even if disposal hangs - Log warning when SSH /home/ethan resolution fails and falls back to /tmp - Add random suffix to test markers to prevent collision
- Test that child processes are terminated when parent is killed (validates set -m) - Test that display_name round-trips through spawn and list
54cd503 to
badae77
Compare
The bash tool blocks commands starting with 'sleep' to prevent wasting time. Changed test commands from 'sleep 30' to 'true && sleep 30' to bypass this validation while still testing background process behavior.
- BashToolCall: show '⚡ background • display_name' for background spawns instead of timeout/duration; show output file paths in expanded view - BashBackgroundListToolCall: new component showing process list with status badges, exit codes, uptimes, scripts, and output file paths - BashBackgroundTerminateToolCall: new component showing terminated process with display_name from result - Wire up new components in ToolMessage.tsx _Generated with mux_
…aths, formatDuration) DRY up tool components by extracting common patterns: - ToolIcon: emoji + tooltip wrapper used in all tools - ErrorBox: danger-styled error display box - OutputPaths: stdout/stderr file path display - formatDuration: human-readable time formatting (ms/s/m/h) Net reduction of ~39 lines in new components while improving consistency.
Update FileReadToolCall, FileEditToolCall, WebFetchToolCall, StatusSetToolCall, and TodoToolCall to use ToolIcon and ErrorBox primitives instead of duplicated TooltipWrapper/Tooltip and manual error styling. Net removal of ~26 lines while ensuring consistent styling.
- Add compact prop to OutputPaths for inline use in cards - Remove redundant empty state in expanded view (header already shows it)
6aabe4a to
144a77f
Compare
### Stack: 1. #923 1. #920 (base) <- This PR --- ## Summary Adds `run_in_background=true` option to the bash tool, enabling agents to spawn long-running processes (dev servers, builds, file watchers) that persist independently. ## Why This Approach We needed background process support that works identically for both local and SSH runtimes. A few considerations drove the design: 1. **PTY-based output doesn't fit remote use cases.** For SSH, maintaining a persistent PTY connection just to let agents read stdout would be fragile and complex. Agents need to search, filter, and tail output—not consume a live stream. 2. **File-based output lets agents use standard tools.** By writing stdout/stderr to files on the target machine, agents can use `tail -f`, `grep`, `head`, etc. to inspect output. We don't need to reimplement these filtering capabilities in our own tooling. 3. **A proper long-lived remote daemon is future work.** Ideally, SSH remotes would have a persistent mux process (or agent binary) that manages background jobs directly. The user's frontend would just connect to it. That's a significant architectural change. This PR provides background shell support without requiring that investment—the file-based approach works today with no remote-side dependencies. ## Architecture ``` AI Tools (bash, bash_background_list, bash_background_terminate) ↓ BackgroundProcessManager (lifecycle, in-memory tracking) ↓ Runtime.spawnBackground() (LocalBaseRuntime / SSHRuntime) ↓ BackgroundHandle (file-based output & status) ↓ backgroundCommands.ts (shared shell builders for Local/SSH parity) ``` ## Key Design Decisions | Decision | Rationale | |----------|-----------| | **File-based output** | Works identically for local and SSH, agents read via `tail`/`cat`/`grep`. | | **set -m + nohup** | Robust process isolation, PID === PGID for clean group termination | | **Workspace-scoped** | Processes tied to workspace, cleaned up on workspace removal | | **Lazy status refresh** | No polling overhead, reads `exit_code` file on list() | | **Cleanup on compaction** | Background processes terminated before compaction, as they're not guaranteed to be included (Claude Code does this too) | ## Tools - `bash(run_in_background=true)` — spawns process, returns `stdout_path`/`stderr_path` - `bash_background_list` — lists processes with status and file paths - `bash_background_terminate` — kills process group (SIGTERM → wait → SIGKILL) ## Output Structure ``` /tmp/mux-bashes/{workspaceId}/{bg-xxx}/ ├── stdout.log # Process stdout ├── stderr.log # Process stderr ├── exit_code # Written by trap on exit └── meta.json # Process metadata ``` ## Technical Details ### Process Spawning The spawn command uses a subshell with job control enabled: ```bash (set -m; nohup bash -c 'WRAPPER_SCRIPT' > stdout.log 2> stderr.log < /dev/null & echo $!) ``` **Key elements:** - `set -m` — Enables bash job control, which makes backgrounded processes become their own process group leader (PID === PGID). This is a bash builtin available on all platforms. - `nohup` — Prevents SIGHUP from killing the process when the terminal closes. - Subshell `(...)` — Isolates the process group so the outer shell exits immediately after echoing the PID. - `< /dev/null` — Detaches stdin so the process doesn't block waiting for input. ### Exit Code Detection The wrapper script sets up a trap to capture the exit code: ```bash trap 'echo $? > exit_code' EXIT && cd /path && export ENV=val && USER_SCRIPT ``` When the process exits (normally or via signal), the trap writes `$?` to the `exit_code` file. Mux reads this file to determine if the process is still running (`exit_code` doesn't exist) or has exited (file contains the code). ### Process Group Termination Because `set -m` ensures PID === PGID, we can kill the entire process tree using a negative PID: ```bash kill -15 -PID; sleep 2; if kill -0 -PID; then kill -9 -PID; echo 137 > exit_code; else echo 143 > exit_code; fi ``` **Sequence:** 1. Send SIGTERM (`-15`) to the process group (`-PID` targets the group) 2. Wait 2 seconds for graceful shutdown 3. Check if any process in the group survives (`kill -0 -PID`) 4. If still alive, send SIGKILL (`-9`) and record exit code 137 5. Otherwise, record exit code 143 (SIGTERM) This ensures child processes spawned by the background job are also terminated, preventing orphaned processes. ### Cross-Platform Compatibility | Feature | Linux | macOS | Windows (MSYS2) | |---------|-------|-------|-----------------| | `set -m` | ✓ bash builtin | ✓ bash builtin | ✓ bash builtin | | `kill -15/-9 -PID` | ✓ | ✓ | ✓ | | `nohup` | ✓ | ✓ | ✓ | | Path format | POSIX | POSIX | Converted via `cygpath` | Using `set -m` instead of platform-specific tools like `setsid` (Linux-only) ensures the same code works everywhere. ## Platform Support - **Linux/macOS/Windows MSYS2**: set -m + nohup pattern (universal) - **SSH**: Same pattern executed remotely ## Testing - 20 unit tests in `backgroundProcessManager.test.ts` (including process group termination) - 7 unit tests for background execution in `bash.test.ts` - 6 unit tests in `bash_background_list.test.ts` (including display_name) - 5 unit tests in `bash_background_terminate.test.ts` - 19 unit tests in `backgroundCommands.test.ts` - 7 runtime tests in `tests/runtime/runtime.test.ts` (Local & SSH) - 3 end-to-end integration tests in `tests/ipc/backgroundBash.test.ts` (real AI calls) _Generated with mux_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds custom UI styling and components for bash background process tools, improving the visual presentation of background process management in the tool interface.
Key Changes:
- Introduced dedicated UI components (
BashBackgroundListToolCall,BashBackgroundTerminateToolCall) for displaying background process information with status badges, uptimes, and output file paths - Refactored common UI primitives (
ToolIcon,ErrorBox,OutputPaths) to reduce code duplication across tool components - Enhanced
BashToolCallto conditionally display background process indicators instead of timeout/duration for background tasks
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/browser/components/tools/shared/toolUtils.tsx | Added formatDuration utility function for human-readable time formatting |
| src/browser/components/tools/shared/ToolPrimitives.tsx | Added reusable ToolIcon, ErrorBox, and OutputPaths components |
| src/browser/components/tools/WebFetchToolCall.tsx | Refactored to use new shared ToolIcon and ErrorBox primitives |
| src/browser/components/tools/TodoToolCall.tsx | Refactored to use new shared ToolIcon primitive |
| src/browser/components/tools/StatusSetToolCall.tsx | Refactored to use new shared ToolIcon primitive |
| src/browser/components/tools/FileReadToolCall.tsx | Refactored to use new shared ToolIcon and ErrorBox primitives |
| src/browser/components/tools/FileEditToolCall.tsx | Refactored to use new shared ToolIcon and ErrorBox primitives |
| src/browser/components/tools/BashToolCall.tsx | Enhanced to conditionally show background indicators and output file paths; uses new formatDuration utility |
| src/browser/components/tools/BashBackgroundTerminateToolCall.tsx | New component displaying terminated background processes with display names |
| src/browser/components/tools/BashBackgroundListToolCall.tsx | New component showing background process list with status badges and details |
| src/browser/components/Messages/ToolMessage.tsx | Added routing logic for new background process tool components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0ea46d3 to
10b3b09
Compare
Stack:
🤖 feat: add UI styling for background bash processes #923 <- This PR
🤖 feat: add background bash process execution with SSH support #920 (base)
Adds custom UI components for background bash tools:
BashToolCall: Shows
⚡ background • display_namefor background spawns instead of timeout/duration; shows output file paths in expanded viewBashBackgroundListToolCall: New component showing process list with status badges, exit codes, uptimes, scripts, and output file paths
BashBackgroundTerminateToolCall: New component showing terminated process with display_name from result
Testing: Manually tested on Linux, Mac, and Windows.
Closes #493
Generated with mux